arrow-flight: generate dict_ids for dicts nested inside complex types#9556
arrow-flight: generate dict_ids for dicts nested inside complex types#9556asubiotto wants to merge 1 commit intoapache:mainfrom
Conversation
github-actions
bot
added
arrow
|
cc @brancz |
Some cases were missing. Signed-off-by: Alfonso Subiotto Marques <alfonso.subiotto@polarsignals.com>
asubiotto
force-pushed
the
asubiotto/ipcid
branch
from
3393972 to
670d9ad
Compare
|
cc @alamb |
alamb
left a comment
alamb
left a comment
There was a problem hiding this comment.
Thank you -- makes sense to me @asubiotto and @brancz
I left some non blocking comments. If you would like to address them let me know otherwise I'll plan to merge this PR tomorrow
| .with_metadata(field.metadata().clone()) | ||
| } | ||
| } | ||
| DataType::ListView(inner) | DataType::LargeListView(inner) => { |
There was a problem hiding this comment.
👍 this makes sense to add handling for these nested types.
There was a problem hiding this comment.
It would probably be good to change the catch all at the bottom of the match statement to explicitly list out all remaining DataTypes as a way to audit that we didn't miss any additional nested types (either now or in the future)
.with_metadata(field.metadata().clone()),
_ => field.as_ref().clone(),
}|
|
||
| let schema = Arc::new(Schema::new(vec
Some cases were missing.
Which issue does this PR close?
Rationale for this change
Fix flight encoding panic
What changes are included in this PR?
Assigning dict ids properly to nested dicts
Are these changes tested?
Yes. The same tests fail on main.
Are there any user-facing changes?